-
Notifications
You must be signed in to change notification settings - Fork 411
Async KV Store #3778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Async KV Store #3778
Conversation
👋 Hi! I see this is a draft PR. |
391ffb1
to
faa071d
Compare
lightning/src/util/async_persist.rs
Outdated
Poll::Pending => { | ||
println!("Future not ready, using tokio runtime"); | ||
|
||
self.runtime.spawn(async move { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be tokio, it could be a generic spawn method too of course.
lightning/src/util/async_persist.rs
Outdated
|
||
match fut.as_mut().poll(&mut cx) { | ||
Poll::Ready(_) => { | ||
UpdateStatus::Completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This initiates the stable persistence flow in LDK.
d57cf75
to
4678f85
Compare
lightning/src/util/persist.rs
Outdated
@@ -134,9 +136,9 @@ pub trait KVStore { | |||
/// | |||
/// Will create the given `primary_namespace` and `secondary_namespace` if not already present | |||
/// in the store. | |||
fn write( | |||
fn write_async( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder: document that these calls, even though they are async, need to complete in the order in which they are initiated.
217d757
to
e551842
Compare
let mut cx = Context::from_waker(&waker); | ||
|
||
match fut.as_mut().poll(&mut cx) { | ||
Poll::Ready(Ok(())) => Ok(true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder - can't simply conclude here that we are in sync mode. An async implementation may also sometimes be ready immediately?
Prepare for adding runtime state while avoiding the _unused serialization macro config.
To prepare for an async kv store trait that must be awaited, this commit moves the kv store calls from the chain notification handlers to the background process. It uses a dirty flag to communicate that there is something to persist. The block height is part of the persisted data. If that data does not make it to disk, the chain notifications are replayed after restart.
3d49db7
to
d31760d
Compare
Draft PR for discussion on approach
Relates to #1470